runtime-sdk: Implement message emission gas#1662
Conversation
✅ Deploy Preview for oasisprotocol-oasis-sdk canceled.
|
kostko
force-pushed
the
kostko/feature/messages-gas
branch
from
2516bb0 to
76cb1e8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1662 +/- ##
==========================================
+ Coverage 65.25% 65.28% +0.03%
==========================================
Files 114 114
Lines 8349 8365 +16
==========================================
+ Hits 5448 5461 +13
- Misses 2901 2904 +3 ☔ View full report in Codecov by Sentry. |
kostko
force-pushed
the
kostko/feature/messages-gas
branch
4 times, most recently
from
7d08b65 to
04c38eb
Compare
|
I've tested this locally with multiple delegations (10 as an example) and it works as expected. Performing 10 delegations this costs 1171870 gas, so with a 15mm gas limit this allows 128 delegations. I have a branch in sapphire-paratime which adds a test for this: oasisprotocol/sapphire-paratime#287 |
CedarMist
left a comment
CedarMist
left a comment
There was a problem hiding this comment.
This works for me, I have not reviewed the code though.
| #[cbor(optional)] | ||
| pub gas: u64, | ||
| /// Maximum amount of emitted consensus messages paid for. | ||
| /// Maximum amount of emitted consensus messages paid for. Zero means that up to the maximum |
There was a problem hiding this comment.
Wouldn't it be better to use max u32 instead of 0 to indicate that up to the maximum number of per-batch messages can be emitted? Maybe we even need zero to disallow consensus messages.
There was a problem hiding this comment.
I think using zero makes it easier to use because this field is already deployed and just omitting the field will result in the new behavior be in effect (which makes the most sense for most use cases). There is no reason to actually have this field now, and we can consider removing it in a later iteration.
Previously one had to explicitly configure the maximum number of messages to consensus layer that can be emitted by a transaction. This posed a problem for Ethereum-compatible transactions which don't support this field. This implementation adds message emission gas which is dynamically calculated based on MAX_BATCH_GAS and MAX_MESSAGES. Similar to storage gas, it is only charged in case other use is too small in order to limit the total number of messages that can be emitted in a batch. All Ethereum-compatible transactions now use this dynamic limit for the maximum number of consensus messages to be emitted.
kostko
force-pushed
the
kostko/feature/messages-gas
branch
from
04c38eb to
bbd26a9
Compare
…ostko/feature/messages-gas runtime-sdk: Implement message emission gas fec2fa9
…/kostko/feature/messages-gas runtime-sdk: Implement message emission gas fec2fa9
3 participants
Previously one had to explicitly configure the maximum number of messages to consensus layer that can be emitted by a transaction. This posed a problem for Ethereum-compatible transactions which don't support this field.
This implementation adds message emission gas which is dynamically calculated based on MAX_BATCH_GAS and MAX_MESSAGES. Similar to storage gas, it is only charged in case other use is too small in order to limit the total number of messages that can be emitted in a batch.
All Ethereum-compatible transactions now use this dynamic limit for the maximum number of consensus messages to be emitted.